Skip to content

fix: Send returnUrl to notes app when shared drive recipient#3746

Merged
zatteo merged 2 commits intomasterfrom
fix/shared-drive-recipient-note-return-url
Mar 19, 2026
Merged

fix: Send returnUrl to notes app when shared drive recipient#3746
zatteo merged 2 commits intomasterfrom
fix/shared-drive-recipient-note-return-url

Conversation

@zatteo
Copy link
Copy Markdown
Member

@zatteo zatteo commented Mar 18, 2026

Required
linagora/cozy-notes#501
linagora/cozy-client#1677

Summary by CodeRabbit

  • New Features

    • Public shared-drive notes now include and preserve a returnUrl parameter, so users are returned to their original location after viewing a shared note.
    • Link generation for shared-drive notes now computes and appends a return URL when available, improving navigation flow.
  • Bug Fixes

    • Redirect handling for public note links now responds to changes in the URL query string.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 18, 2026

Walkthrough

PublicNoteRedirect now reads the location search string (via useLocation), extracts a returnUrl query parameter, and passes it to fetchURL; the effect re-runs when search changes. computePath gained a client option and updated signature; for public shared-drive notes, when driveId and client exist it appends an encoded returnUrl produced by a new makeSharedDriveNoteReturnUrl helper. A new helper builds a shared-drive web return URL using CozyClient. Tests and a dependency bump to cozy-client were updated accordingly.

Possibly related PRs

Suggested reviewers

  • rezk2ll
  • lethemanh
  • doubleface
  • JF-Cozy
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main objective of sending a returnUrl to the notes app for shared drive recipients, which aligns with the code changes across multiple files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/shared-drive-recipient-note-return-url
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

codescene-delta-analysis[bot]

This comment was marked as outdated.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
src/modules/navigation/PublicNoteRedirect.tsx (1)

26-60: Consider adding search to the useEffect dependency array.

The search variable is used inside the effect (line 32) but isn't listed in the dependency array. While this is likely fine since search won't change during this redirect flow, adding it would satisfy the exhaustive-deps rule and prevent potential stale closure issues if the component's usage changes.

♻️ Add search to dependencies
-  }, [fileId, driveId, client])
+  }, [fileId, driveId, client, search])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/navigation/PublicNoteRedirect.tsx` around lines 26 - 60, The
useEffect in PublicNoteRedirect (the effect that defines fetchNoteUrl)
references the variable search but does not include it in the dependency array;
update the dependency list for that useEffect to include search (so the effect
depends on [fileId, driveId, client, search]) to satisfy exhaustive-deps and
avoid stale closures when search changes.
src/modules/shareddrives/helpers.ts (1)

43-57: Non-null assertion on file.driveId could mask runtime issues.

The function uses file.driveId! but the type IOCozyFile doesn't guarantee driveId is present. While the caller in computePath checks for driveId before calling, this function doesn't enforce that precondition.

Consider either:

  1. Adding a runtime check with a descriptive error
  2. Using a more specific type that guarantees driveId
🛡️ Suggested defensive check
 export const makeSharedDriveNoteReturnUrl = (
   client: CozyClient,
   file: IOCozyFile
 ): string => {
+  if (!file.driveId) {
+    throw new Error('driveId is required to generate shared drive return URL')
+  }
   return generateWebLink({
     slug: 'drive',
     searchParams: [],
     // eslint-disable-next-line `@typescript-eslint/no-unsafe-member-access`
     cozyUrl: client.getStackClient().uri as string,
     // eslint-disable-next-line `@typescript-eslint/no-unsafe-assignment`
     subDomainType: client.getInstanceOptions().subdomain,
     pathname: '',
-    hash: `/shareddrive/${file.driveId!}/${file.dir_id}`
+    hash: `/shareddrive/${file.driveId}/${file.dir_id}`
   })
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/shareddrives/helpers.ts` around lines 43 - 57, The helper
makeSharedDriveNoteReturnUrl is using a non-null assertion on file.driveId which
can mask runtime errors; update it to either (a) perform a runtime check at the
start of makeSharedDriveNoteReturnUrl and throw a descriptive error if
file.driveId is missing, or (b) tighten the function signature to accept a file
type that guarantees driveId (e.g., IOCozyFile & { driveId: string }) so callers
like computePath must pass a file with driveId; ensure references to
client.getStackClient(), client.getInstanceOptions(), and the hash construction
remain unchanged.
src/modules/navigation/hooks/helpers.ts (1)

120-130: Empty returnUrl parameter appended when client is null.

When driveId exists but client is null, the path becomes /note/${driveId}/${file._id}?returnUrl= with an empty value. Consider omitting the query parameter entirely when returnUrl is empty to produce cleaner URLs.

♻️ Cleaner URL when returnUrl is empty
     case 'public-note':
       if (driveId) {
         const returnUrl = client
           ? makeSharedDriveNoteReturnUrl(client, file as IOCozyFile)
           : ''

-        return `/note/${driveId}/${file._id}?returnUrl=${encodeURIComponent(
-          returnUrl
-        )}`
+        const basePath = `/note/${driveId}/${file._id}`
+        return returnUrl
+          ? `${basePath}?returnUrl=${encodeURIComponent(returnUrl)}`
+          : basePath
       } else {
         return `/note/${file._id}`
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/navigation/hooks/helpers.ts` around lines 120 - 130, The
generated URL includes an empty returnUrl query when client is null; in the
block that constructs the shared-drive note path (inside
src/modules/navigation/hooks/helpers.ts around the code using driveId, file._id
and makeSharedDriveNoteReturnUrl), only append "?returnUrl=..." when returnUrl
is non-empty/truthy — i.e., compute returnUrl with
makeSharedDriveNoteReturnUrl(client, file) and then branch to return
`/note/${driveId}/${file._id}` if returnUrl is falsy, otherwise return
`/note/${driveId}/${file._id}?returnUrl=${encodeURIComponent(returnUrl)}` so no
empty query parameter is emitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/modules/navigation/PublicNoteRedirect.tsx`:
- Line 63: Remove the debug console.log call that prints noteUrl in
PublicNoteRedirect.tsx (the line console.log('🔴 noteUrl', noteUrl)); locate it
inside the PublicNoteRedirect component where noteUrl is created/used and delete
that statement, or replace it with a proper structured logging call if
persistent metrics are needed (e.g., use the app logger or debug flag) so no
debug console output remains in production.

In `@src/modules/shareddrives/helpers.ts`:
- Line 55: The hash generation uses file.dir_id directly which can be undefined
and produce "undefined" in the URL; update the code that builds the hash (the
template literal where hash: `/shareddrive/${file.driveId!}/${file.dir_id}`) to
validate file.dir_id first and use a safe fallback (e.g., empty string or a
designated "root" token) or conditionally omit the segment so the produced path
never contains the literal "undefined"; ensure the same validation pattern is
applied wherever file.dir_id is used for URL/path construction.

---

Nitpick comments:
In `@src/modules/navigation/hooks/helpers.ts`:
- Around line 120-130: The generated URL includes an empty returnUrl query when
client is null; in the block that constructs the shared-drive note path (inside
src/modules/navigation/hooks/helpers.ts around the code using driveId, file._id
and makeSharedDriveNoteReturnUrl), only append "?returnUrl=..." when returnUrl
is non-empty/truthy — i.e., compute returnUrl with
makeSharedDriveNoteReturnUrl(client, file) and then branch to return
`/note/${driveId}/${file._id}` if returnUrl is falsy, otherwise return
`/note/${driveId}/${file._id}?returnUrl=${encodeURIComponent(returnUrl)}` so no
empty query parameter is emitted.

In `@src/modules/navigation/PublicNoteRedirect.tsx`:
- Around line 26-60: The useEffect in PublicNoteRedirect (the effect that
defines fetchNoteUrl) references the variable search but does not include it in
the dependency array; update the dependency list for that useEffect to include
search (so the effect depends on [fileId, driveId, client, search]) to satisfy
exhaustive-deps and avoid stale closures when search changes.

In `@src/modules/shareddrives/helpers.ts`:
- Around line 43-57: The helper makeSharedDriveNoteReturnUrl is using a non-null
assertion on file.driveId which can mask runtime errors; update it to either (a)
perform a runtime check at the start of makeSharedDriveNoteReturnUrl and throw a
descriptive error if file.driveId is missing, or (b) tighten the function
signature to accept a file type that guarantees driveId (e.g., IOCozyFile & {
driveId: string }) so callers like computePath must pass a file with driveId;
ensure references to client.getStackClient(), client.getInstanceOptions(), and
the hash construction remain unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7d81f709-9055-4f6c-80b6-784720d35bea

📥 Commits

Reviewing files that changed from the base of the PR and between 5ecfcb4 and f917fbd.

📒 Files selected for processing (4)
  • src/modules/navigation/PublicNoteRedirect.tsx
  • src/modules/navigation/hooks/helpers.ts
  • src/modules/navigation/hooks/useFileLink.tsx
  • src/modules/shareddrives/helpers.ts

Comment thread src/modules/navigation/PublicNoteRedirect.tsx Outdated
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
subDomainType: client.getInstanceOptions().subdomain,
pathname: '',
hash: `/shareddrive/${file.driveId!}/${file.dir_id}`
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

file.dir_id is used without validation.

Similar to driveId, dir_id could be undefined (e.g., for root-level items), which would result in a malformed URL containing the literal string "undefined".

🛡️ Add validation for dir_id
 export const makeSharedDriveNoteReturnUrl = (
   client: CozyClient,
   file: IOCozyFile
 ): string => {
+  if (!file.driveId || !file.dir_id) {
+    throw new Error('driveId and dir_id are required to generate shared drive return URL')
+  }
   return generateWebLink({
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/shareddrives/helpers.ts` at line 55, The hash generation uses
file.dir_id directly which can be undefined and produce "undefined" in the URL;
update the code that builds the hash (the template literal where hash:
`/shareddrive/${file.driveId!}/${file.dir_id}`) to validate file.dir_id first
and use a safe fallback (e.g., empty string or a designated "root" token) or
conditionally omit the segment so the produced path never contains the literal
"undefined"; ensure the same validation pattern is applied wherever file.dir_id
is used for URL/path construction.

@zatteo zatteo force-pushed the fix/shared-drive-recipient-note-return-url branch from f917fbd to 936dc4c Compare March 18, 2026 06:56
codescene-delta-analysis[bot]

This comment was marked as outdated.

@zatteo zatteo force-pushed the fix/shared-drive-recipient-note-return-url branch from 936dc4c to 14e2a5f Compare March 18, 2026 07:00
codescene-delta-analysis[bot]

This comment was marked as outdated.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/modules/shareddrives/helpers.ts (1)

43-57: ⚠️ Potential issue | 🟡 Minor

Validate driveId and dir_id before constructing the URL.

Both file.driveId and file.dir_id are used without validation. While driveId uses a non-null assertion (!), and the caller in computePath checks for driveId, dir_id could still be undefined (e.g., for root-level items), resulting in a malformed URL containing the literal string "undefined".

🛡️ Add validation for both properties
 export const makeSharedDriveNoteReturnUrl = (
   client: CozyClient,
   file: IOCozyFile
 ): string => {
+  if (!file.driveId || !file.dir_id) {
+    throw new Error('driveId and dir_id are required to generate shared drive return URL')
+  }
   return generateWebLink({
     slug: 'drive',
     searchParams: [],
     // eslint-disable-next-line `@typescript-eslint/no-unsafe-member-access`
     cozyUrl: client.getStackClient().uri as string,
     // eslint-disable-next-line `@typescript-eslint/no-unsafe-assignment`
     subDomainType: client.getInstanceOptions().subdomain,
     pathname: '',
-    hash: `/shareddrive/${file.driveId!}/${file.dir_id}`
+    hash: `/shareddrive/${file.driveId}/${file.dir_id}`
   })
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/shareddrives/helpers.ts` around lines 43 - 57, In
makeSharedDriveNoteReturnUrl validate that file.driveId and file.dir_id are
defined before building the hash: if driveId is missing throw/return an explicit
error (do not rely on the non-null assertion), and for dir_id handle root-level
items by using an explicit fallback (e.g. empty string or a canonical "root"
token) so the generated hash never contains the literal "undefined"; update the
hash construction (inside generateWebLink call in makeSharedDriveNoteReturnUrl)
to use the validated driveId and a safe dirId fallback.
🧹 Nitpick comments (1)
src/modules/navigation/PublicNoteRedirect.tsx (1)

32-33: Consider validating returnUrl before passing to fetchURL to prevent potential open redirect vulnerabilities.

Although returnUrl typically originates from the controlled makeSharedDriveNoteReturnUrl() function, it's extracted directly from URL search parameters which users can modify. The parameter is passed to fetchURL without validation. Consider adding a check to ensure returnUrl points to a legitimate Cozy instance before passing it along.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/navigation/PublicNoteRedirect.tsx` around lines 32 - 33, Validate
the extracted returnUrl from searchParams before passing it to fetchURL in
PublicNoteRedirect: parse returnUrl using the URL constructor (or a safe
parser), ensure its origin/host matches an allowed Cozy instance pattern (e.g.,
exact trusted host(s), allowed host suffix, or match window.location.origin if
appropriate), and only call fetchURL(returnUrl) when the check passes; otherwise
fall back to a safe internal URL or abort the redirect. Update the logic around
searchParams/returnUrl and any call sites of fetchURL to perform this validation
so untrusted user-supplied returnUrl values cannot trigger open redirects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/modules/navigation/hooks/helpers.ts`:
- Around line 120-130: The URL builder in the block referencing driveId, client,
makeSharedDriveNoteReturnUrl and file currently always appends a returnUrl query
parameter (empty when client is null); change the logic so you only add
?returnUrl=... when returnUrl is a non-empty string: compute returnUrl via
makeSharedDriveNoteReturnUrl(client, file) only if client exists, then build
`/note/${driveId}/${file._id}` and append
`?returnUrl=${encodeURIComponent(returnUrl)}` only when returnUrl is truthy
(omit the entire query string when it is empty or undefined).

---

Duplicate comments:
In `@src/modules/shareddrives/helpers.ts`:
- Around line 43-57: In makeSharedDriveNoteReturnUrl validate that file.driveId
and file.dir_id are defined before building the hash: if driveId is missing
throw/return an explicit error (do not rely on the non-null assertion), and for
dir_id handle root-level items by using an explicit fallback (e.g. empty string
or a canonical "root" token) so the generated hash never contains the literal
"undefined"; update the hash construction (inside generateWebLink call in
makeSharedDriveNoteReturnUrl) to use the validated driveId and a safe dirId
fallback.

---

Nitpick comments:
In `@src/modules/navigation/PublicNoteRedirect.tsx`:
- Around line 32-33: Validate the extracted returnUrl from searchParams before
passing it to fetchURL in PublicNoteRedirect: parse returnUrl using the URL
constructor (or a safe parser), ensure its origin/host matches an allowed Cozy
instance pattern (e.g., exact trusted host(s), allowed host suffix, or match
window.location.origin if appropriate), and only call fetchURL(returnUrl) when
the check passes; otherwise fall back to a safe internal URL or abort the
redirect. Update the logic around searchParams/returnUrl and any call sites of
fetchURL to perform this validation so untrusted user-supplied returnUrl values
cannot trigger open redirects.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9dbbfeef-f03f-4ec8-8e01-e169937fe304

📥 Commits

Reviewing files that changed from the base of the PR and between f917fbd and 936dc4c.

📒 Files selected for processing (4)
  • src/modules/navigation/PublicNoteRedirect.tsx
  • src/modules/navigation/hooks/helpers.ts
  • src/modules/navigation/hooks/useFileLink.tsx
  • src/modules/shareddrives/helpers.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/modules/navigation/hooks/useFileLink.tsx

Comment on lines +120 to +130
if (driveId) {
const returnUrl = client
? makeSharedDriveNoteReturnUrl(client, file as IOCozyFile)
: ''

return `/note/${driveId}/${file._id}?returnUrl=${encodeURIComponent(
returnUrl
)}`
} else {
return `/note/${file._id}`
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Empty returnUrl query parameter when client is null.

When driveId exists but client is null, returnUrl becomes an empty string, resulting in a URL like /note/{driveId}/{fileId}?returnUrl=. This empty query parameter may be unintended and could cause issues in the notes app when parsing.

Consider only appending the returnUrl parameter when it has a value.

🔧 Conditionally append returnUrl
     case 'public-note':
       if (driveId) {
         const returnUrl = client
           ? makeSharedDriveNoteReturnUrl(client, file as IOCozyFile)
           : ''

-        return `/note/${driveId}/${file._id}?returnUrl=${encodeURIComponent(
-          returnUrl
-        )}`
+        const basePath = `/note/${driveId}/${file._id}`
+        return returnUrl
+          ? `${basePath}?returnUrl=${encodeURIComponent(returnUrl)}`
+          : basePath
       } else {
         return `/note/${file._id}`
       }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (driveId) {
const returnUrl = client
? makeSharedDriveNoteReturnUrl(client, file as IOCozyFile)
: ''
return `/note/${driveId}/${file._id}?returnUrl=${encodeURIComponent(
returnUrl
)}`
} else {
return `/note/${file._id}`
}
if (driveId) {
const returnUrl = client
? makeSharedDriveNoteReturnUrl(client, file as IOCozyFile)
: ''
const basePath = `/note/${driveId}/${file._id}`
return returnUrl
? `${basePath}?returnUrl=${encodeURIComponent(returnUrl)}`
: basePath
} else {
return `/note/${file._id}`
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/navigation/hooks/helpers.ts` around lines 120 - 130, The URL
builder in the block referencing driveId, client, makeSharedDriveNoteReturnUrl
and file currently always appends a returnUrl query parameter (empty when client
is null); change the logic so you only add ?returnUrl=... when returnUrl is a
non-empty string: compute returnUrl via makeSharedDriveNoteReturnUrl(client,
file) only if client exists, then build `/note/${driveId}/${file._id}` and
append `?returnUrl=${encodeURIComponent(returnUrl)}` only when returnUrl is
truthy (omit the entire query string when it is empty or undefined).

@bundlemon
Copy link
Copy Markdown

bundlemon bot commented Mar 18, 2026

BundleMon

Files updated (6)
Status Path Size Limits
static/js/main.(hash).js
163.98KB (+127B +0.08%) -
public/static/js/public.(hash).js
138.18KB (+105B +0.07%) -
public/static/js/cozy.(hash).js
787.38KB (-13B 0%) -
static/js/cozy.(hash).js
922.16KB (-13B 0%) -
services/dacc.js
263.14KB (-20B -0.01%) -
services/qualificationMigration.js
283.41KB (-25B -0.01%) -
Unchanged files (15)
Status Path Size Limits
static/js/(chunkId).(hash).js
1.07MB -
public/static/js/(chunkId).(hash).js
1.01MB -
(hash).js
336.11KB -
public/(hash).js
336.11KB -
public/static/js/lib-react.(hash).js
43.88KB -
static/js/lib-react.(hash).js
43.88KB -
public/static/css/cozy.(hash).css
33.87KB -
static/css/cozy.(hash).css
33.87KB -
public/static/js/lib-router.(hash).js
21.86KB -
static/js/lib-router.(hash).js
21.86KB -
static/css/main.(hash).css
16.99KB -
public/static/css/public.(hash).css
6.72KB -
manifest.webapp
3.04KB -
index.html
705B -
assets/manifest.json
185B -

Total files change +163B 0%

Groups updated (1)
Status Path Size Limits
**/*.js
7.43MB (+140B 0%) -
Unchanged groups (2)
Status Path Size Limits
**/*.{png,svg,ico}
2.19MB -
**/*.css
128.57KB -

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
src/modules/navigation/hooks/helpers.ts (1)

120-130: ⚠️ Potential issue | 🟡 Minor

Empty returnUrl query parameter when client is null.

When driveId exists but client is null, the code appends an empty returnUrl parameter (e.g., /note/drive456/note123?returnUrl=). This could cause issues in the notes app when parsing the URL.

Consider only appending the parameter when returnUrl has a value:

🔧 Conditionally append returnUrl
     case 'public-note':
       if (driveId) {
         const returnUrl = client
           ? makeSharedDriveNoteReturnUrl(client, file as IOCozyFile)
           : ''

-        return `/note/${driveId}/${file._id}?returnUrl=${encodeURIComponent(
-          returnUrl
-        )}`
+        const basePath = `/note/${driveId}/${file._id}`
+        return returnUrl
+          ? `${basePath}?returnUrl=${encodeURIComponent(returnUrl)}`
+          : basePath
       } else {
         return `/note/${file._id}`
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/navigation/hooks/helpers.ts` around lines 120 - 130, The code
builds a note URL that includes a returnUrl query param even when client is
null, producing an empty `returnUrl=`; update the logic in the block that checks
`driveId` (the function using `driveId`, `client`,
`makeSharedDriveNoteReturnUrl`, and `file._id`) to only append `?returnUrl=...`
when `returnUrl` is non-empty: compute `returnUrl` via
`makeSharedDriveNoteReturnUrl(client, file)` only if `client` is truthy, then
conditionally add the `?returnUrl=encodeURIComponent(returnUrl)` suffix to
`/note/${driveId}/${file._id}` only when `returnUrl` has a value; otherwise
return the base `/note/${driveId}/${file._id}` without any empty query
parameter.
src/modules/shareddrives/helpers.ts (1)

43-57: ⚠️ Potential issue | 🟡 Minor

Validate driveId and dir_id before use to prevent malformed URLs.

The static analysis flagged the non-null assertion on file.driveId! (line 55). While the caller in computePath checks for driveId existence, dir_id could still be undefined for root-level items, resulting in a URL containing the literal string "undefined".

Consider adding validation at the function boundary for defensive programming:

🛡️ Proposed fix with validation
 export const makeSharedDriveNoteReturnUrl = (
   client: CozyClient,
   file: IOCozyFile
 ): string => {
+  if (!file.driveId || !file.dir_id) {
+    throw new Error('driveId and dir_id are required to generate shared drive return URL')
+  }
   return generateWebLink({
     slug: 'drive',
     searchParams: [],
     // eslint-disable-next-line `@typescript-eslint/no-unsafe-member-access`
     cozyUrl: client.getStackClient().uri as string,
     // eslint-disable-next-line `@typescript-eslint/no-unsafe-assignment`
     subDomainType: client.getInstanceOptions().subdomain,
     pathname: '',
-    hash: `/shareddrive/${file.driveId!}/${file.dir_id}`
+    hash: `/shareddrive/${file.driveId}/${file.dir_id}`
   })
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/shareddrives/helpers.ts` around lines 43 - 57, The
makeSharedDriveNoteReturnUrl function uses file.driveId! and file.dir_id
directly, which can produce malformed URLs if either is undefined; update this
function to validate file.driveId and file.dir_id at the start (e.g., if
(!file.driveId || !file.dir_id) throw a descriptive Error or normalize dir_id to
''/root as appropriate for your app), remove the non-null assertion on
file.driveId, and then build the hash using the validated/normalized values when
calling generateWebLink so the URL never contains the literal "undefined".
🧹 Nitpick comments (1)
src/modules/navigation/hooks/helpers.spec.js (1)

237-242: Consider adding test coverage for the case when client is provided.

This test validates the behavior when client is not provided (empty returnUrl). Adding a test case that mocks a CozyClient and verifies the actual return URL generation would improve coverage of the happy path.

💡 Suggested additional test case
it('should return correct path for public-note with driveId and client', () => {
  const file = { _id: 'note123', driveId: 'drive456', dir_id: 'folder789' }
  const mockClient = {
    getStackClient: () => ({ uri: 'https://example.mycozy.cloud' }),
    getInstanceOptions: () => ({ subdomain: 'nested' })
  }
  const result = computePath(file, {
    type: 'public-note',
    pathname: '/public',
    client: mockClient
  })
  expect(result).toMatch(/^\/note\/drive456\/note123\?returnUrl=/)
  expect(result).toContain(encodeURIComponent('https://'))
})

Note: You may need to mock generateWebLink from cozy-client for this test to work properly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/navigation/hooks/helpers.spec.js` around lines 237 - 242, Add a
unit test that exercises computePath when a CozyClient-like `client` is
provided: create a file object with _id, driveId (and optional dir_id), mock a
client exposing getStackClient and getInstanceOptions (or stub generateWebLink
from cozy-client), call computePath(file, { type: 'public-note', pathname:
'/public', client: mockClient }) and assert the returned path still matches the
/note/:driveId/:id?returnUrl=... pattern and that the query contains a non-empty
encoded returnUrl (e.g., contains "https://" when decoded or contains
encodeURIComponent('https://')), ensuring the test covers the happy path with
client present; use the same test file and helper names (computePath,
generateWebLink) to locate code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/modules/navigation/hooks/helpers.ts`:
- Around line 120-130: The code builds a note URL that includes a returnUrl
query param even when client is null, producing an empty `returnUrl=`; update
the logic in the block that checks `driveId` (the function using `driveId`,
`client`, `makeSharedDriveNoteReturnUrl`, and `file._id`) to only append
`?returnUrl=...` when `returnUrl` is non-empty: compute `returnUrl` via
`makeSharedDriveNoteReturnUrl(client, file)` only if `client` is truthy, then
conditionally add the `?returnUrl=encodeURIComponent(returnUrl)` suffix to
`/note/${driveId}/${file._id}` only when `returnUrl` has a value; otherwise
return the base `/note/${driveId}/${file._id}` without any empty query
parameter.

In `@src/modules/shareddrives/helpers.ts`:
- Around line 43-57: The makeSharedDriveNoteReturnUrl function uses
file.driveId! and file.dir_id directly, which can produce malformed URLs if
either is undefined; update this function to validate file.driveId and
file.dir_id at the start (e.g., if (!file.driveId || !file.dir_id) throw a
descriptive Error or normalize dir_id to ''/root as appropriate for your app),
remove the non-null assertion on file.driveId, and then build the hash using the
validated/normalized values when calling generateWebLink so the URL never
contains the literal "undefined".

---

Nitpick comments:
In `@src/modules/navigation/hooks/helpers.spec.js`:
- Around line 237-242: Add a unit test that exercises computePath when a
CozyClient-like `client` is provided: create a file object with _id, driveId
(and optional dir_id), mock a client exposing getStackClient and
getInstanceOptions (or stub generateWebLink from cozy-client), call
computePath(file, { type: 'public-note', pathname: '/public', client: mockClient
}) and assert the returned path still matches the
/note/:driveId/:id?returnUrl=... pattern and that the query contains a non-empty
encoded returnUrl (e.g., contains "https://" when decoded or contains
encodeURIComponent('https://')), ensuring the test covers the happy path with
client present; use the same test file and helper names (computePath,
generateWebLink) to locate code.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1d75d4cf-fce0-4c4b-87d6-b5b090a0152d

📥 Commits

Reviewing files that changed from the base of the PR and between 936dc4c and 14e2a5f.

📒 Files selected for processing (5)
  • src/modules/navigation/PublicNoteRedirect.tsx
  • src/modules/navigation/hooks/helpers.spec.js
  • src/modules/navigation/hooks/helpers.ts
  • src/modules/navigation/hooks/useFileLink.tsx
  • src/modules/shareddrives/helpers.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/modules/navigation/PublicNoteRedirect.tsx

@zatteo zatteo force-pushed the fix/shared-drive-recipient-note-return-url branch from 14e2a5f to 7eb3a8a Compare March 19, 2026 07:13
Copy link
Copy Markdown

@codescene-delta-analysis codescene-delta-analysis bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gates Passed
3 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
src/modules/navigation/hooks/helpers.ts (1)

120-127: ⚠️ Potential issue | 🟡 Minor

Avoid emitting an empty returnUrl query param

When client is null, this still generates ?returnUrl=. Build the base note URL first, and only append returnUrl when it is non-empty.

Suggested fix
     case 'public-note':
       if (driveId) {
         const returnUrl = client
           ? makeSharedDriveNoteReturnUrl(client, file as IOCozyFile)
           : ''

-        return `/note/${driveId}/${file._id}?returnUrl=${encodeURIComponent(
-          returnUrl
-        )}`
+        const basePath = `/note/${driveId}/${file._id}`
+        return returnUrl
+          ? `${basePath}?returnUrl=${encodeURIComponent(returnUrl)}`
+          : basePath
       } else {
         return `/note/${file._id}`
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/modules/navigation/hooks/helpers.ts` around lines 120 - 127, The URL
builder emits an empty returnUrl query param when client is null; update the
logic in the block that uses driveId, file, and makeSharedDriveNoteReturnUrl so
you first construct the base path `/note/${driveId}/${file._id}` and then, only
if client is truthy and makeSharedDriveNoteReturnUrl(client, file as IOCozyFile)
returns a non-empty string, append
`?returnUrl=${encodeURIComponent(returnUrl)}`; ensure you do not append
`?returnUrl=` when returnUrl is empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/modules/navigation/hooks/helpers.ts`:
- Around line 120-127: The URL builder emits an empty returnUrl query param when
client is null; update the logic in the block that uses driveId, file, and
makeSharedDriveNoteReturnUrl so you first construct the base path
`/note/${driveId}/${file._id}` and then, only if client is truthy and
makeSharedDriveNoteReturnUrl(client, file as IOCozyFile) returns a non-empty
string, append `?returnUrl=${encodeURIComponent(returnUrl)}`; ensure you do not
append `?returnUrl=` when returnUrl is empty.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5a476cbe-bbec-4360-811c-fdb03d985060

📥 Commits

Reviewing files that changed from the base of the PR and between 14e2a5f and 7eb3a8a.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (6)
  • package.json
  • src/modules/navigation/PublicNoteRedirect.tsx
  • src/modules/navigation/hooks/helpers.spec.js
  • src/modules/navigation/hooks/helpers.ts
  • src/modules/navigation/hooks/useFileLink.tsx
  • src/modules/shareddrives/helpers.ts
✅ Files skipped from review due to trivial changes (2)
  • src/modules/navigation/hooks/helpers.spec.js
  • package.json
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/modules/navigation/PublicNoteRedirect.tsx
  • src/modules/shareddrives/helpers.ts
  • src/modules/navigation/hooks/useFileLink.tsx

@zatteo zatteo merged commit 4ff07ba into master Mar 19, 2026
6 checks passed
@zatteo zatteo deleted the fix/shared-drive-recipient-note-return-url branch March 19, 2026 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants